-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support llvm 17 #187
support llvm 17 #187
Conversation
That is quite a large diff. Is that all necessary, or are there formatting changes that can be separated? |
Yeah, formatting changes can be separated, there are only few necessary changes. |
That would be helpful then, as this is too big to open and will conflict with everyone's open work, so we probably do not want to merge any unnecessary format changes |
Thanks for your reply, I format it with clang-format again and the diff is much smaller now. |
Seems like more clang-format fixes are still needed? |
Linux and windows tests are passed now. The macos tests failed due to the warnings introduced during compilation of some test cases. Maybe more flags should be introduced here. |
Looks like it used to be disabled in the distant past: llvm-cbe/tools/llvm-cbe/llvm-cbe.cpp Lines 306 to 314 in 2ca604e
I don't know what might be wrong about the current output: llvm-cbe/lib/Target/CBackend/CBackend.cpp Lines 3878 to 3880 in b1efd16
Do you think you could update the test so that it dumps the contents of the generated file to stderr if the test fails? |
I dump the generated code for the failure case and play with it on my mac. It seems that the warning is introduced by To reproduce the warning, you can compile try compile the following simple example.
I think it would be safe to remove the Having the above warning fixed, there are still two warnings regarding uninitialized variables, which is introduced by phi node. |
Tests passed, the version of gcc is different in the github actions :) |
No description provided.